Repository navigation
Fix cherry-pick changing the source commit's metadata - #10592
Merged
nopcoder merged 1 commit intoNov 5, 2026
Merged
Conversation
Signed-off-by: Ryo Kusnadi <ryokusnadi@gmail.com>
github-actions
Bot
added
area/cataloger
Contributor
Author
|
@nopcoder friendly ping on this one when you have a moment. Happy to change anything. |
nopcoder
self-requested a review
November 5, 2026 13:47
Contributor
|
Thanks, @RyoKusnadi. From a quick look, it seems we have the same issue in Import as well. This can be addressed in a separate PR. I'll be looking into making the required checks pass and will merge this one. |
Contributor
Author
|
Thanks! I'll check the Import path and open a separate PR for it if it's the same bug. |
nopcoder
approved these changes
Nov 5, 2026
Contributor
Author
|
@nopcoder Following up on Import: I checked it and it doesn't have the same problem. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #10565
Change Description
Bug Fix
How it happens: cherry-pick a commit that has commit metadata (for example one made with
"metadata":{"k":"a"}on the commits API) onto another branch. The server then reads that source commit from its commit cache and changes its metadata in memory, so the original commit's ID stops resolving:GET /repositories/rep/commits/<id>returnscommit not found. A commit with no metadata is not affected. Steps are in the issue.POST .../branches/main/cherry-pickwith a source commit that has metadata, that source commit can no longer be fetched by its ID (commit not found). Passingmetadatain the cherry-pick overrides also changes the map the caller passed in.Graveler.CherryPicksetcommit.Metadata = commitRecord.Metadata, or the override map, and then wrotecherry-pick-originandcherry-pick-committerinto it. That is the same map the cached source commit holds (ref.Manager.GetCommitreturns one shared*Commit), so the cached source commit gained the two keys.maps.Clonebefore adding the two keys, so the new commit gets its own map. I did this at the write site instead of copying when reading the source commit, because the override map has the same problem.Testing Details
TestGraveler_CherryPickDoesNotModifySourceCommitMetadata: cherry-picks a commit with metadata{"k":"a"}and checks the source commit's metadata is unchanged. Before the fix it fails (the source commit gainscherry-pick-originandcherry-pick-committer); after, it passes. It uses the in-memory ref manager fake, so it does not touch the real commit cache.TestGraveler_CherryPickHooks,TestGravelerCherryPick/cherry-pick_partially_override_commit_fields) compared the new commit's metadata to the same map they passed in, so they only passed because the map was being changed. They now check the expected keys and that the passed map stays as it was.Run:
go test ./pkg/graveler/...,go vet ./pkg/graveler/,gofmt -l pkg/graveler. Not run: the full repo suite, the system tests, or the curl steps against a live server.Breaking Change?
No. The new commit has the same metadata as before.
Additional info
Same pattern elsewhere: the merge path (
graveler.go,MergeStrategyMetadataKey) also writes intocommitParams.Metadata. That map comes from the request, not from a cached commit, so I left it out of this PR.Contact Details
@RyoKusnadi